Skip to content

fix(windows): repair PATHEXT in spawned shells + ESM ripgrep wrapper#484

Merged
wonderwhy-er merged 2 commits into
mainfrom
fix/windows-support
Jun 1, 2026
Merged

fix(windows): repair PATHEXT in spawned shells + ESM ripgrep wrapper#484
wonderwhy-er merged 2 commits into
mainfrom
fix/windows-support

Conversation

@wonderwhy-er

@wonderwhy-er wonderwhy-er commented May 31, 2026

Copy link
Copy Markdown
Owner

Summary

Two Windows-specific fixes, both surfaced while investigating #481.

Investigates: #481

1. Repair corrupted PATHEXT before spawning shells (the #481 report)

On some Windows Claude Desktop / DXT launches, the Desktop Commander server
process inherits a broken PATHEXT (observed as .CPL only). Because
terminal-manager.ts builds each child shell's environment from
{ ...process.env }, that broken value propagated into every spawned shell,
stripping .EXE and breaking resolution of git / node / python / rg
(and even full-path .exe invocations under PowerShell).

Findings that refine the original report:

  • Not ARM64-specific — reproduced on AMD64/x64 as well. It affects the
    Windows DXT v0.2.41 launch path generally.
  • DC's own code never sets PATHEXT; the value is inherited from how the DXT
    host launches the server, then faithfully copied to children. Root cause is
    upstream (Claude Desktop's DXT launcher), but DC is the right place to defend
    since it controls the env handed to child shells.

Fix: on win32, repair PATHEXT immediately before spawn. If .EXE is
missing, merge the standard extension list with whatever was present
(non-destructive when PATHEXT is already healthy).

2. Make the bundled ripgrep wrapper ESM

@vscode/ripgrep 1.18.0+ ships package.json with "type": "module". The
MCPB build copies scripts/ripgrep-wrapper.js over the package's
lib/index.js, but the wrapper was CommonJS (require / module.exports), so
it threw require is not defined in ES module scope on import. That throw was
swallowed by getRipgrepPath()'s try/catch and surfaced as a misleading
ripgrep binary not found, silently breaking start_search on Windows even
though the bundled binaries were present.

Fix: rewrite the wrapper as ESM (import + export const rgPath,
__dirname via fileURLToPath). Verified with a fresh Node import against the
deployed extension: rgPath resolves to rg-x86_64-pc-windows-msvc.exe and the
binary exists.

Testing

  • tsc --noEmit: no new type errors from these changes.
  • ripgrep wrapper: validated via fresh import() against the bundled package on
    Windows x64 — named rgPath export resolves and the target binary exists.
  • PATHEXT: confirmed the broken .CPL value at process level on x64, and that
    restoring PATHEXT makes git/node/etc. resolve again.

Notes

  • Both fixes are Windows-only in effect; non-Windows behavior is unchanged.
  • The PATHEXT issue likely also warrants a parallel report to Claude Desktop's
    DXT launcher, but this PR lets DC self-heal without waiting on that.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Windows shell command resolution issues caused by corrupted inherited environment variables.
  • Improvements

    • Enhanced ripgrep binary discovery with improved fallback logic for platform-specific binaries and clearer error messages when unavailable.

On some Windows Claude Desktop/DXT launches the server process inherits a broken PATHEXT (observed as '.CPL' only). Since child shells are spawned with { ...process.env }, that value propagated into every shell, stripping '.EXE' and breaking resolution of git/node/python/rg and even full-path .exe invocations under PowerShell.

Repair PATHEXT on win32 just before spawn: if '.EXE' is missing, merge the standard list with whatever was present (non-destructive otherwise). Reproduces on x64 too, not only ARM64.

Refs #481
…module)

@vscode/ripgrep 1.18.0+ ships package.json with 'type: module'. The MCPB build copies scripts/ripgrep-wrapper.js over the package's lib/index.js, but it was CommonJS (require/module.exports), so it threw 'require is not defined in ES module scope' on import.

That throw was swallowed by getRipgrepPath()'s try/catch and surfaced as a misleading 'ripgrep binary not found', silently breaking search on Windows despite the bundled binaries being present. Rewrite the wrapper as ESM (import + export const rgPath, __dirname via fileURLToPath).

Refs #481
@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: daf362cd-4057-493e-bc43-42103968dd9b

📥 Commits

Reviewing files that changed from the base of the PR and between 9c44119 and 9ad0bca.

📒 Files selected for processing (2)
  • scripts/ripgrep-wrapper.js
  • src/terminal-manager.ts

📝 Walkthrough

Walkthrough

This PR migrates the ripgrep wrapper from CommonJS to ES modules with improved binary resolution fallback logic, and adds Windows-specific PATHEXT environment variable repair to prevent command resolution failures in spawned shell processes when the inherited PATHEXT is corrupted.

Changes

Windows Command Execution Reliability

Layer / File(s) Summary
ESM migration and ripgrep binary resolution
scripts/ripgrep-wrapper.js
Converts from CommonJS require/module.exports to ESM import/export. Derives __dirname via fileURLToPath(import.meta.url). Refactors binary path resolution to check platform-specific binary first (rg-[platform]), fall back to generic rg/rg.exe, and throw a descriptive error if neither exists. Preserves Unix chmodSync(0o755) behavior to ensure executable permissions.
Windows PATHEXT environment repair
src/terminal-manager.ts
Adds a standard PATHEXT constant and a helper function that detects missing .EXE extension in inherited PATHEXT and merges it with the standard list while preserving extras. Applies the repaired PATHEXT to spawnOptions.env on Windows before spawning child processes to prevent command resolution failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #481: Windows PATHEXT corruption in spawned processes directly addressed by terminal-manager PATHEXT repair logic.
  • #480: PATHEXT inheritance problem directly resolved by the new Windows environment variable repair helper.

Possibly related PRs

Suggested labels

size:M

Suggested reviewers

  • serg33v

Poem

🐰 A ripgrep wrapper dons new ESM clothes,
While PATHEXT healers mend Windows woes,
Platform binaries fallback with care,
Spawned shells breathe clean Unix air,
Commands resolve without despair! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: fixing PATHEXT on Windows and converting the ripgrep wrapper to ESM, matching the core objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-support

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wonderwhy-er wonderwhy-er merged commit 673111c into main Jun 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants